Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Typing][A-54,A-55,A-56] Add type annotations for paddle/nn/functional/{activation.py, common.py, conv.py} #65191

Merged
merged 8 commits into from
Jun 19, 2024

Conversation

gsq7474741
Copy link
Contributor

@gsq7474741 gsq7474741 commented Jun 14, 2024

PR Category

User Experience

PR Types

Improvements

Description

类型标注:

  • paddle/nn/functional/activation.py
  • paddle/nn/functional/common.py
  • paddle/nn/functional/conv.py

Related links

@SigureMo @megemini

Copy link

paddle-bot bot commented Jun 14, 2024

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot paddle-bot bot added the contributor External developers label Jun 14, 2024
def softmax(
x: Tensor,
axis: int = -1,
dtype: Literal['bfloat16', 'float16', 'float32', 'float64'] | None = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不支持 paddle.dtype 形式么?支持的话统一使用 DTypeLike

Copy link
Contributor Author

@gsq7474741 gsq7474741 Jun 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里没说支持int和complex和bool

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是没关系的,我们不可能每次将所有支持的数据类型都枚举出来,这部分工作只能由运行时来做,至少我们需要保证该支持的传进来类型检查不会报错,目前这类 API 应该统一使用 DTypeLike

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修改下这里就没其它的问题了,这里明显是支持 paddle.dtype / np.dtype

if (
(dtype is not None)
and (not isinstance(dtype, core.VarDesc.VarType))
and (not isinstance(dtype, core.DataType))
):
dtype = convert_np_dtype_to_dtype_(dtype)

def softmax_(
x: Tensor,
axis: int = -1,
dtype: Literal['bfloat16', 'float16', 'float32', 'float64'] | None = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里没说支持int和complex和bool

def log_softmax(
x: Tensor,
axis: int = -1,
dtype: Literal['float32', 'float64'] | None = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上

python/paddle/nn/functional/common.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/common.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/conv.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/conv.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/conv.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/conv.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/conv.py Outdated Show resolved Hide resolved
Copy link
Member

@SigureMo SigureMo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#65197 发现了 DataLayout2D 里的一个 typo,可以麻烦先改一下么?

-DataLayout2D: TypeAlias = Literal["NCHW", "NHCW"]
+DataLayout2D: TypeAlias = Literal["NCHW", "NHWC"]

@gsq7474741
Copy link
Contributor Author

DataLayout2D: TypeAlias = Literal["NCHW", "NHWC"]

夜猫子好评 >_<

@SigureMo SigureMo added the HappyOpenSource 快乐开源活动issue与PR label Jun 17, 2024
@megemini
Copy link
Contributor

@gsq7474741 CI 里面 fail 的,看看是不是 numpy 版本与 opencv 兼容问题 ~ 可以重新构建一下 ~

python/paddle/nn/functional/activation.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/activation.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/activation.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/activation.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/common.py Show resolved Hide resolved
x: Tensor,
size: ShapeLike | None = None,
scale_factor: ShapeLike | None = None,
mode: Literal[
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

放到 _typing 里面吧,加个类似 PaddingMode 的类型,叫做 InterpolateMode 吧 ~ 其他地方也会用到 ~

另外,Literal["NCW", "NWC", "NCHW", "NHWC", "NCDHW", "NDHWC"] 这个也改成 DataLayoutNDVariant 吧,类似 DataLayout1DVariant ~ 在 _typing.layout.py 下面 ~

@SigureMo 后面是不是可以反馈一下,把这个统一一下 ~

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

确认下其他地方有没有用,没有用的话不建议一股脑丢到 _typing,部分类型可以按照模块本来的 import 关系管理,比如 functional 里定义一个 TypeAlias,layer 里引用,那就完全没必要塞到 _typing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不多,就这几个
image

] = 'nearest',
align_corners: bool = False,
align_mode: int = 0,
data_format: Literal["NCW", "NWC", "NCHW", "NHWC", "NCDHW", "NDHWC"] = None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上~

def pad(
x: Tensor,
pad: ShapeLike,
mode: Literal["constant", "reflect", "replicate", "circular"] = 'constant',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

刚翻了一下 numpy 的处理方式,numpy 是把这类东西叫做 _ModeKind ,私有的,所以放到各个模块中 ~

由此,后面类似的处理方式可以有两种:

  • numpy 的方式
  • 统一放到 _typing 中,但是名称有区分

如果采用后者,这个也放到 _typing 里面,感觉这个叫 PaddingMode, conv 里面的那个叫 ConvPaddingMode ~ 如何?

@SigureMo 帮忙看看

python/paddle/nn/functional/common.py Outdated Show resolved Hide resolved
@gsq7474741
Copy link
Contributor Author

@SigureMo 这个不算PaddlePaddle Hackathon

@SigureMo
Copy link
Member

@SigureMo 这个不算PaddlePaddle Hackathon

我不确定,我看之前的子任务都是用的 HappyOpenSource 快乐开源活动issue与PR

@luotao1 确认下?

@gsq7474741 不过这个会有什么影响吗?

Copy link
Member

@SigureMo SigureMo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTMeow 🐾

@SigureMo SigureMo merged commit 5b69171 into PaddlePaddle:develop Jun 19, 2024
32 of 33 checks passed
co63oc pushed a commit to co63oc/Paddle that referenced this pull request Jun 25, 2024
…al/{activation.py, common.py, conv.py}` (PaddlePaddle#65191)


---------

Co-authored-by: SigureMo <sigure.qaq@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource 快乐开源活动issue与PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants